Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

400 returning when calling elasticsearch from feign #424

Closed
jaylucas opened this issue Jul 29, 2016 · 31 comments
Closed

400 returning when calling elasticsearch from feign #424

jaylucas opened this issue Jul 29, 2016 · 31 comments
Labels
bug Unexpected or incorrect behavior

Comments

@jaylucas
Copy link

jaylucas commented Jul 29, 2016

So I have a basic request to a locally running instance of elasticsearch

    @RequestLine("GET /_search?q=body:{body}")
    public ElasticGetResponse getResumes(@Param("body") String body);

Here is what I get through the logger:

[ElasticSearchService#getResumes] ---> GET http://localhost:9200/_search?q=body%3A%7Bbody%7D HTTP/1.1
[ElasticSearchService#getResumes] Content-Length: 21
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {
  "body" : "phil"
}
[ElasticSearchService#getResumes] ---> END HTTP (21-byte body)
[ElasticSearchService#getResumes] <--- HTTP/1.1 400 Bad Request (154ms)
[ElasticSearchService#getResumes] Content-Length: 470
[ElasticSearchService#getResumes] Content-Type: application/json; charset=UTF-8
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {"error":{"root_cause":    [{"type":"search_parse_exception","reason":"failed to parse search source. unknown search element [body]","line":2,"col":3}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"1","node":"rOGZzEghRVacdIRh_E9Wog","reason":{"type":"search_parse_exception","reason":"failed to parse search source. unknown search element [body]","line":2,"col":3}}]},"status":400}
[ElasticSearchService#getResumes] <--- END HTTP (470-byte body)

Now of course I dont get the problem when I curl the same endpoint

curl 'http://localhost:9200/_search?q=body:phil'

returns the correct docs

My question is why does feign hate that colon? When I have _search?q=phil it works....

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Jul 29, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Jul 29, 2016

So this is what I got with the curl -v

* Connected to localhost (127.0.0.1) port 9200 (#0)
> GET /_search?q=body:phil HTTP/1.1
> User-Agent: curl/7.37.1
> Host: localhost:9200
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: application/json; charset=UTF-8
< Content-Length: 5503 
   * Connection #0 to host localhost left intact
  {"took":4,"timed_out":false,"_shards": .........

I tried using wire shark to view the url but unfortunately I suck at wireshark. I can see the packets (SYN, ACK, FIN) but I can't determine the url past 127.0.0.1 buuuut looking at the logs you can see the destination addr is messed up anyway example:

When I have @RequestLine("GET /_search?q={body}") I get on this line: https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/SynchronousMethodHandler.java#L88

SynchronousMethodHandler.java
     Object executeAndDecode(RequestTemplate template) throws Throwable {
             Request request = targetRequest(template); //<------ here my request URL == http://localhost:9200/_search?q=phil

So at this point I already have the correct url.

But with @RequestLine("GET /_search?q=body:{body}") I get at that same line:

  request.url = http://localhost:9200/_search?q=body%3A%7Bbody%7D

So what is happening is that feign is not injecting the the variable (in this case phil) into the @RequestLine("GET /_search?q=body:{body}") seemingly because of the colon?

it turns into %3A%7B which I am assuming is :{ then %7D or }

Any idea?

@jaylucas
Copy link
Author

jaylucas commented Aug 3, 2016

Any word on this?

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 3, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Aug 3, 2016

:(

@codefromthecrypt
Copy link
Contributor

ok looking at your dump, it looks like a request body is being created, eventhough this is a GET request! This sounds very strange to me.

@codefromthecrypt
Copy link
Contributor

ex the below is showing a json body being made. The code you pasted doesn't seem like it should be creating a json body. Is there anything else (perhaps about how feign is constructed), which might explain this?

[ElasticSearchService#getResumes] ---> GET http://localhost:9200/_search?q=body%3A%7Bbody%7D HTTP/1.1
[ElasticSearchService#getResumes] Content-Length: 21
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {
  "body" : "phil"
}

@jaylucas
Copy link
Author

jaylucas commented Aug 3, 2016

This is what I have:

public class ElasticSearchFactory {
   public static ElasticSearchService elasticSearchService() {
        final String host = "http://localhost:9200";

        return Feign.builder()
            .encoder(new JacksonEncoder())
            .decoder(new JacksonDecoder())
            .logger(new Logger.JavaLogger().appendToFile("/logs/http.log"))
            .logLevel(Logger.Level.FULL)
            .target(ElasticSearchService.class, host);
     }

    public interface ElasticSearchService {

       @RequestLine("POST /document/{id}")
        public ElasticPostResponse addInfo(@Param("id") Integer id, Document doc);

       @RequestLine("GET /_search?q={body}")
        public ElasticGetResponse workingGetInfo(@Param("body") String body);

       @RequestLine("GET /_search?q=body:{body}")
       public ElasticGetResponse brokenGetInfo(@Param("body") String body);
   }
}

Creating it in another class.

private ElasticSearchFactory.ElasticSearchService
        elasticSearchService = ElasticSearchFactory.elasticSearchService();

Then calling it like this:

ElasticGetResponse elasticGetResponse = elasticSearchService.getInfo(query);

query being a string

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 4, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Aug 4, 2016

Thanks for the help!!

@codefromthecrypt
Copy link
Contributor

This is a bug for sure. Not sure yet which of the various recent changes caused it.

  interface Issue424 {
    @RequestLine("GET /_search?q=body:{body}")
    String get(@Param("body") String body);
  }

  @Test
  public void getDoesntImplyFormParams() throws Exception {
    List<MethodMetadata> mds = contract.parseAndValidatateMetadata(Issue424.class);
    assertThat(mds.get(0).formParams()).isEmpty();
  }

@codefromthecrypt
Copy link
Contributor

actually, I see what's going on.

We added support for partial-match on headers, and not on queries. Ex. the {body} part is a partial-match on the query param q.

codefromthecrypt pushed a commit that referenced this issue Aug 4, 2016
This is to help develop apis like Elasticsearch which nest queries in
query parameters.

Fixes #424
@codefromthecrypt
Copy link
Contributor

#428

@jaylucas
Copy link
Author

jaylucas commented Aug 5, 2016

This is great thank you

spencergibb pushed a commit that referenced this issue Aug 5, 2016
This is to help develop apis like Elasticsearch which nest queries in
query parameters.

Fixes #424
@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 5, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Aug 16, 2016

Sorry for the super late response. I was away of vacation.

But I am still getting the same issue

[ElasticSearchService#getResumes] ---> GET http://localhost:9200/_search?q=body%3A%7Bbody%7D HTTP/1.1
[ElasticSearchService#getResumes] Content-Length: 20
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {
  "body" : "jay"
}
[ElasticSearchService#getResumes] ---> END HTTP (20-byte body)
[ElasticSearchService#getResumes] <--- HTTP/1.1 400 Bad Request (54ms)
[ElasticSearchService#getResumes] content-length: 470
[ElasticSearchService#getResumes] content-type: application/json; charset=UTF-8
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {"error":{"root_cause":    [{"type":"search_parse_exception","reason":"failed to parse search source. unknown search element [body]","line":2,"col":3}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"1","node":"MWwTeNKxTfGi4QewR6zb9g","reason":{"type":"search_parse_exception","reason":"failed to parse search source. unknown search element [body]","line":2,"col":3}}]},"status":400}
[ElasticSearchService#getResumes] <--- END HTTP (470-byte body)

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 16, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Aug 16, 2016

Lol I edited my response. I thought It was different then before.

https://mvnrepository.com/artifact/com.netflix.feign/feign-core/8.18.0

I can even go to the line with the edits. So yes.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 16, 2016 via email

@jaylucas
Copy link
Author

jaylucas commented Aug 16, 2016

Yeah I tried this too:
https://mvnrepository.com/artifact/io.github.openfeign/feign-core/9.1.0
using gradle so

compile group: 'io.github.openfeign', name: 'feign-core', version: '9.1.0'

@jaylucas
Copy link
Author

jaylucas commented Aug 16, 2016

So the actual log with 9.1.0 is:

[ElasticSearchService#getResumes] ---> GET http://localhost:9200/_search?q=body%3A%7Bbody%7D HTTP/1.1
[ElasticSearchService#getResumes] ---> END HTTP (0-byte body)
[ElasticSearchService#getResumes] <--- HTTP/1.1 400 Bad Request (53ms)
[ElasticSearchService#getResumes] content-length: 2190
[ElasticSearchService#getResumes] content-type: application/json; charset=UTF-8
[ElasticSearchService#getResumes] 
[ElasticSearchService#getResumes] {"error":{"root_cause":[{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"1"},{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"document"},{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"test"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"1","node":"MWwTeNKxTfGi4QewR6zb9g","reason":{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"1","caused_by":{"type":"parse_exception","reason":"Cannot parse 'body:{body}': Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    "}}}},{"shard":0,"index":"document","node":"MWwTeNKxTfGi4QewR6zb9g","reason":{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"document","caused_by":{"type":"parse_exception","reason":"Cannot parse 'body:{body}': Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    "}}}},{"shard":0,"index":"test","node":"MWwTeNKxTfGi4QewR6zb9g","reason":{"type":"query_parsing_exception","reason":"Failed to parse query [body:{body}]","index":"test","caused_by":{"type":"parse_exception","reason":"Cannot parse 'body:{body}': Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    ","caused_by":{"type":"parse_exception","reason":"Encountered \" \"}\" \"} \"\" at line 1, column 10.\nWas expecting one of:\n    \"TO\" ...\n    <RANGE_QUOTED> ...\n    <RANGE_GOOP> ...\n    "}}}}]},"status":400}
[ElasticSearchService#getResumes] <--- END HTTP (2190-byte body)

There is no { "jay" }
Sorry its late and im making mistakes - mixed up my logs

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 16, 2016 via email

@codefromthecrypt
Copy link
Contributor

Failing test:

  @Test
  public void resolveTemplateWithParameterizedSubstringQueryValue() {
    RequestTemplate template = new RequestTemplate().method("GET")
        .append("_search").query("q", "body:{body}");

    template.resolve(mapOf("body", "text"));

    assertThat(template)
        .hasQueries(entry("q", asList("body:text")));
  }

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 16, 2016

@jaylucas might be a couple days to fix.. here's a workaround that would work if you have only one input to the "q" parameter:

    @RequestLine("GET /_search?q={q}")
    public ElasticGetResponse getResumes(@Param(value = "q", expander = PrefixBodyExpander.class) String body);

  static final class PrefixBodyExpander implements Param.Expander {
    @Override
    public String expand(Object value) {
      return "body:" + value;
    }
  }

@jaylucas
Copy link
Author

jaylucas commented Aug 16, 2016

Yeah I figured. Thanks for the update.

I actually have been doing that while this issue is being resolved. The problem with that is when dealing with ids. let say:

/_search?q=1

This searches all occurrences of 1 which can be many more times then the unique id im looking for

/_search?q=id:1

Grabs the 1 value Im looking for

@codefromthecrypt
Copy link
Contributor

id:1 would still work with the janky workaround :) Anyway glad you have a workaround.. should have this sorted in a few days.

 static final class PrefixIdExpander implements Param.Expander {
    @Override
    public String expand(Object value) {
      return "id:" + value;
    }
  }

@jaylucas
Copy link
Author

Thank you 👍

@DevAwas
Copy link

DevAwas commented Oct 9, 2017

Is this sorted yet? I am unable to run my client like:
@headers(HttpHeaders.CONTENT_TYPE + ":" + MediaType.APPLICATION_JSON_VALUE)
@RequestLine("GET /search?query:{query}")
Map getProductDetails(@param(value = "query") String query);

I run into NPE at Caused by: java.lang.NullPointerException: null
at feign.Contract$Default.searchMapValuesContainsSubstring(Contract.java:291)
at feign.Contract$Default.processAnnotationsOnParameter(Contract.java:264)
at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:107)
at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64)
at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146)
at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53)
at feign.Feign$Builder.target(Feign.java:218)
at org.springframework.cloud.netflix.feign.HystrixTargeter.target(HystrixTargeter.java:39)

I am on spring-cloud-starter-feign 1.3.2.RELEASE(feign-core 9.5.0)

@DevAwas
Copy link

DevAwas commented Oct 9, 2017

ignore my query, i was passing incorrectly, sorry!

@slepasteur
Copy link

Any new on that? I try calling an API with a request looking like ".../test?param=value={v}" but the "{v}" is not replaced (using version 9.5.1).

@RequestLine ("GET /test?param=value={v}")
void test(@Param ("v") String v);

@kdavisk6 kdavisk6 added the bug Unexpected or incorrect behavior label Jul 20, 2018
@kdavisk6
Copy link
Member

@slepasteur
Your RequestLine is incorrect. You will need to fix the query string before the substitution will work:

@RequestLine ("GET /test?param=&value={v}")
void test(@Param ("v") String v);

Second, this issue should be fixed for most cases outlined. I'm going to close this issue. Please create a new issue, including a test case if you can, if you continue to have trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

5 participants